src/main/java/org/example/projektarendehantering/application/service/DocumentService.java (1)

54-56: Consider unifying closed-case behavior across document endpoints.

uploadDocument/listDocuments treat closed cases as “not found”, while other paths throw “Case is closed”. A single policy would make client handling more predictable.

Also applies to: 117-119, 184-186

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/application/service/DocumentService.java`
around lines 54 - 56, The uploadDocument and listDocuments flows use
caseRepository.findById(...).filter(ce -> ce.getStatus() !=
CaseStatus.CLOSED).orElseThrow(() -> new BadRequestException("Case not found"))
while other code paths throw a "Case is closed" error; unify them: pick a single
policy (e.g. throw new BadRequestException("Case is closed") or a dedicated
CaseClosedException) and apply it consistently in uploadDocument, listDocuments
and the other locations that check CaseStatus (references: CaseEntity,
caseRepository.findById(...).filter(...), and the methods uploadDocument and
listDocuments); replace the differing orElseThrow/assert messages so every
closed-case check throws the same exception type and message so clients see a
predictable response.

src/main/java/org/example/projektarendehantering/application/service/CaseService.java (1)

168-173: Using peek() with authorization check may cause confusing behavior.

If a user requests cases for a patient but lacks access to some of them, requireCanRead will throw on the first unauthorized case, failing the entire request. Consider whether the intent is:

1. All-or-nothing (current behavior): User must have access to all cases for the patient
2. Filter to accessible: Return only cases the user can read

If option 2 is intended:

♻️ Alternative: filter instead of fail

     `@Transactional`(readOnly = true)
     public List<CaseDTO> getCasesForPatient(Actor actor, UUID patientId) {
         return caseRepository.findAllByPatient_IdAndStatusNot(patientId, CaseStatus.CLOSED).stream()
-                .peek(entity -> requireCanRead(actor, entity))
+                .filter(entity -> canRead(actor, entity))
                 .map(caseMapper::toDTO)
                 .collect(Collectors.toList());
     }
+
+    private boolean canRead(Actor actor, CaseEntity entity) {
+        if (isManager(actor)) return true;
+        if (isDoctor(actor) && actor.userId().equals(entity.getOwnerId())) return true;
+        if (isNurse(actor) && actor.userId().equals(entity.getHandlerId())) return true;
+        return false;
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/application/service/CaseService.java`
around lines 168 - 173, The current getCasesForPatient uses peek(requireCanRead)
which throws on the first unauthorized case (all-or-nothing); if we want to
return only cases the actor can read, replace the peek with a filter that
retains entities the actor may read (e.g., call a non-throwing canRead(actor,
entity) predicate or wrap requireCanRead in a try/catch and return true/false)
before mapping with caseMapper::toDTO; update or add a canRead helper if needed
and keep caseRepository.findAllByPatient_IdAndStatusNot and caseMapper::toDTO
intact.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java (1)

45-46: Add assertions for new audit behavior, not just mock wiring.

Since AuditService is now injected, add tests verifying auditService.record(...) is called when status transitions to COMMUNICATION, and not called when status is already COMMUNICATION.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java`
around lines 45 - 46, DocumentServiceTest currently only wires the injected
AuditService mock but doesn't assert behavior; add two tests in
DocumentServiceTest that exercise the method that changes status (the same call
already used in tests) and verify auditService.record(...) is invoked exactly
once when transitioning from a non-COMMUNICATION status to COMMUNICATION, and
verify auditService.record(...) is not invoked when the document is already in
COMMUNICATION. Use Mockito.verify(auditService, times(1)).record(...) with the
expected args for the first case and Mockito.verify(auditService,
never()).record(any()) for the second case (or verifyZeroInteractions/reset the
mock as needed) to make the assertions precise.

src/test/java/org/example/projektarendehantering/presentation/rest/CaseControllerTest.java (1)

188-210: Add a non-manager access test for /api/cases/closed.

The new tests cover manager success and anonymous access, but not authenticated non-manager access (e.g., DOCTOR). Add that case to lock authorization behavior.

🧪 Suggested test addition

+    `@Test`
+    `@WithMockUser`(roles = "DOCTOR")
+    void getClosedCases_shouldBeDenied_forNonManager() throws Exception {
+        mockMvc.perform(get("/api/cases/closed"))
+                .andExpect(status().isForbidden());
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/org/example/projektarendehantering/presentation/rest/CaseControllerTest.java`
around lines 188 - 210, Add a test that authenticates a non-manager user (e.g.,
Actor with Role.DOCTOR) and asserts that calling the controller endpoint
/api/cases/closed returns a forbidden response; specifically, add a test similar
to getClosedCases_shouldReturnList_forManager that stubs
securityActorAdapter.currentUser() to return a DOCTOR Actor and then perform
get("/api/cases/closed") expecting status().isForbidden(), ensuring
CaseService.getClosedCases is not invoked (or not stubbed) in this scenario to
verify authorization enforcement.

src/main/java/org/example/projektarendehantering/presentation/web/UiController.java (1)

55-59: Guard the closed-cases UI route at the controller boundary.

Line 55 should be explicitly restricted to managers so direct URL access is blocked before service invocation.

🔧 Proposed change

 import org.springframework.http.HttpStatus;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.stereotype.Controller;
@@
     `@GetMapping`("/ui/cases/closed")
+    `@PreAuthorize`("hasRole('MANAGER')")
     public String listClosedCases(Model model) {
         model.addAttribute("cases", caseService.getClosedCases(securityActorAdapter.currentUser()));
         return "cases/closed";
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/presentation/web/UiController.java`
around lines 55 - 59, The GET handler listClosedCases in UiController must be
restricted to managers at the controller boundary: add a method-level security
annotation (e.g. `@PreAuthorize`("hasRole('MANAGER')") or
`@Secured`("ROLE_MANAGER")) to the listClosedCases method to block direct URL
access before calling
caseService.getClosedCases(securityActorAdapter.currentUser()); ensure method
security is enabled in your security config (e.g. prePostEnabled=true or
equivalent) so the annotation is enforced.

src/main/java/org/example/projektarendehantering/presentation/rest/CaseController.java (1)

43-46: Add explicit controller-level authorization for closed-cases endpoint.

The service layer already checks for MANAGER role via isManager(actor), but adding @PreAuthorize("hasRole('MANAGER')") at the controller level provides fail-fast denial before the service is invoked and makes the authorization requirement explicit in the API contract.

🔧 Proposed change

 import org.springframework.http.ResponseEntity;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.*;
@@
-    `@GetMapping`("/closed")
+    `@GetMapping`("/closed")
+    `@PreAuthorize`("hasRole('MANAGER')")
     public ResponseEntity<List<CaseDTO>> getClosedCases() {
         return ResponseEntity.ok(caseService.getClosedCases(securityActorAdapter.currentUser()));
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/projektarendehantering/presentation/rest/CaseController.java`
around lines 43 - 46, The closed-cases endpoint lacks an explicit
controller-level authorization annotation; add
`@PreAuthorize`("hasRole('MANAGER')") to the getClosedCases handler (or apply the
same annotation at the controller class level) so requests are fail-fast denied
before calling caseService.getClosedCases and
securityActorAdapter.currentUser(); target the getClosedCases method in
CaseController to implement this change.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes